Conversation
appgurueu
added
@ Script API
Maintenance
| local t = type(o) | ||
| if not level and t == "userdata" then | ||
| -- when userdata (e.g. player) is passed directly, print its metatable: | ||
| return "userdata metatable: " .. dump(getmetatable(o)) |
There was a problem hiding this comment.
I removed this undocumented "feature". I think if modders want this, they should do dump(getmetatable(value)) explicitly.
There was a problem hiding this comment.
this was an explicitly requested feature: #6842
re "undocumented": documenting the precise behavior of a pure debugging helper function isn't needed and can only cause us problems in the future.
There was a problem hiding this comment.
this was an explicitly requested feature: #6842
i wonder whether @HybridDog would still consider this feature a good idea today?
documenting the precise behavior of a pure debugging helper function isn't needed and can only cause us problems in the future.
sure. i'm just trying to argue that i should be somewhat free to remove things like this if i can make a half-decent case that they should be removed.
ultimately i don't mind putting it back in terribly much but i still think it shouldn't work like that. dump({player}, "") gives you {<userdata>,} but dump(player, "") dumps you the (usually annoyingly big) metatable? that's just confusing if you ask me, it breaks the natural recursive nature of dump and adds a weird special case.
we should probably give our userdata proper __tostring implementations which identify it. e.g. PlayerRef: singleplayer or something.
There was a problem hiding this comment.
Hmm yeah if it doesn't work recursively I agree it's not ideal.
There was a problem hiding this comment.
I think if a modder uses dump() on a variable to find out what it is, an output like userdata: 0x7acd0162a3e0 is not very helpful to him/her and showing the metatable is also not ideal. Removing the feature to print the metatable is fine for me.
Adding __tostring implementations for userdata sounds like a better solution if this is possible.
|
I think having |
| local t = type(o) | ||
| if not level and t == "userdata" then | ||
| -- when userdata (e.g. player) is passed directly, print its metatable: | ||
| return "userdata metatable: " .. dump(getmetatable(o)) |
There was a problem hiding this comment.
I think if a modder uses dump() on a variable to find out what it is, an output like userdata: 0x7acd0162a3e0 is not very helpful to him/her and showing the metatable is also not ideal. Removing the feature to print the metatable is fine for me.
Adding __tostring implementations for userdata sounds like a better solution if this is possible.
|
Added a helpful |
sfan5
left a comment
sfan5
left a comment
There was a problem hiding this comment.
I guess.
(added no squash due to the nice separation of commits)
3 participants
I figured it would be nice if the default engine-way of inspecting values was a bit nicer.
Goals:
dumpoutput should be easy to read.dumped strings should produce equivalent values.dumpshould be reasonably efficient (linear time in the size of output), but there is not much of a need to squeeze out constant factors.With this PR:
tostringed if it doesn't lose precision, otherwise they're formatted exactly.setrefandgetref, it is even possible to just copy-paste thedumpoutput and run it as Lua code - see the unit tests.)How to test
Unit tests are included, but feel free to churn more things through
dump.